API V2 discontinued - Changes to migrate to V3#31
API V2 discontinued - Changes to migrate to V3#31martarodriguezm wants to merge 18 commits intowronglink:masterfrom
Conversation
…anslate" function
wronglink
left a comment
wronglink
left a comment
There was a problem hiding this comment.
Need some minor code style fixes and I'd also simplify the API as translator methods combine both array and not array requests.
| self.assertIsInstance(t, dict) | ||
| self.assertIn('Translations', t) | ||
| ts = self.translator.translate_array2(['The answer lies in machine translation.'], 'en', 'es') | ||
| translations = [t['translations'][0]['text'] for t in ts] |
There was a problem hiding this comment.
I'd also like more if we check the full results in ts, not the only first one.
mstranslator.py
Outdated
| 'texts': json.dumps(texts), | ||
| } | ||
| return self._translate('TranslateArray', params, lang_from, lang_to, | ||
| body = [ |
There was a problem hiding this comment.
As I can see from docs, there is no difference between Translate and TranslateArray methods now. Shouldn't we combine methods with _array suffix into one?
There was a problem hiding this comment.
| body = [ | |
| def _array(self, texts=[], lang_from=None, lang_to=None, contenttype='text/plain', category='general', include_alignment=False): | |
| json = [ | |
| {'Text' : text} for text in texts | |
| ] | |
| return self._translate('translate', json, lang_from, lang_to, contenttype, category, include_alignment) | |
| def translate_array(self, texts=[], lang_from=None, lang_to=None, | |
| contenttype='text/plain', category='general'): | |
| return self._array(self, texts, lang_from, lang_to, contenttype, category) | |
| def translate_array2(self, texts=[], lang_from=None, lang_to=None, | |
| contenttype='text/plain', category='general'): | |
| return self._array(self, texts, lang_from, lang_to, contenttype, category, include_alignment=True) |
Do yo mean something like this?
| c = 0 | ||
| result = [] | ||
| for i in lengths: | ||
| for i in lengths[0]['sentLen']: |
There was a problem hiding this comment.
There are different names at doc page. Docs say it's sentLen and example says it's sentenceLengths. What is the actual behaviour?
There was a problem hiding this comment.
'sentLen' is the correct one, I think the docs example is wrong or has not been changed from a previous version.
mstranslator.py
Outdated
| headers = { | ||
| 'Ocp-Apim-Subscription-Key': self.subscription_key | ||
| 'Ocp-Apim-Subscription-Key': self.subscription_key, | ||
| 'Content-type': 'application/json' |
There was a problem hiding this comment.
It's actually not required as we use json= parameter in requests. But if we keep it here, it would be better to leave a comma at the end of string:
'Content-type': 'application/json',
There was a problem hiding this comment.
Not sure about this, I'm not used to use Python, why a comma at the end?
There was a problem hiding this comment.
It's a good habit as python allows to use trailing comma for cleaner diffs. If there was a trailing comma at after self.subscription_key before, we would see only one line added (not one changed and one added).
There was a problem hiding this comment.
Ok, I see the point. I have just added.
|
Hi, Have you had time to review the changes I submitted? Thanks! |
|
Hi, Could be possible to accept those changes so the Translator Text API can continue working? Thank you! |
3 participants
Translator Text API V2 is going to be discontinued, I have done the migration to V3.
This Pull Request fixes #30